-
Notifications
You must be signed in to change notification settings - Fork 236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolves #81 - Reduce log levels #92
Conversation
* Reduced all `log.Error*` events to `log.Debug*` in `exporter.go` where an existing internal metric tracks the event.
@@ -274,7 +274,7 @@ func (b *Exporter) Listen(e <-chan Events) { | |||
// We don't accept negative values for counters. Incrementing the counter with a negative number | |||
// will cause the exporter to panic. Instead we will warn and continue to the next event. | |||
if event.Value() < 0.0 { | |||
log.Errorf("Counter %q is: '%f' (counter must be non-negative value)", metricName, event.Value()) | |||
log.Debugf("Counter %q is: '%f' (counter must be non-negative value)", metricName, event.Value()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one doesn't seem to have a metric associated with it. But, I don't think it's that important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add such a metric/error counter before reducing the log level though. Silently dropping metrics would be very annoying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM besides the missing metric.
@@ -274,7 +274,7 @@ func (b *Exporter) Listen(e <-chan Events) { | |||
// We don't accept negative values for counters. Incrementing the counter with a negative number | |||
// will cause the exporter to panic. Instead we will warn and continue to the next event. | |||
if event.Value() < 0.0 { | |||
log.Errorf("Counter %q is: '%f' (counter must be non-negative value)", metricName, event.Value()) | |||
log.Debugf("Counter %q is: '%f' (counter must be non-negative value)", metricName, event.Value()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add such a metric/error counter before reducing the log level though. Silently dropping metrics would be very annoying.
* Added internal metric to track `illegal_negative_counter` events
@grobie See updated PR branch, added internal metric as requested |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
log.Error*
events tolog.Debug*
inexporter.go
wherean existing internal metric tracks the event.